Skip to content

Conversation

@kamko
Copy link
Contributor

@kamko kamko commented Dec 21, 2021

all info in #7

@pimterry
Copy link
Member

pimterry commented Jan 3, 2022

Just to update here - I am still looking into this, but I haven't fixed it yet.

The interesting bit is that -keep class tech.httptoolkit.relocated.** { *; } isn't sufficient. That means there's some low-level detail of the transformation that r8 is doing that breaks something. That's not super surprising (I can imagine all sorts of changes which shouldn't affect any normal usage but do affect bytebuddy somehow) but it's quite inconvenient.

We can work around this by editing the include/exclude rules in the distJar gradle task (which copies in specific original untouched class files on top of the r8 output) and that works, but it's a tradeoff to try and do that for the smallest compatible set of files required to make this work.

I do have a working patch, by excluding net.bytebuddy.** and org.objectweb.asm.** entirely from r8, and adding an include/exclude for just tech/httptoolkit/relocated/net/bytebuddy/agent/**/* to the gradle distJar build. That leaves us with a dist jar of 3MB though, which is a fair jump on the 1.75MB JARs we've had until now. This compares to 4.6MB for the unminified build. I'm also not 100% sure why this works yet, which i don't love.

At the end of the day, it's not a disaster to push the size up, and maybe we'll have to, but that extra weight goes directly onto the HTTP Toolkit download for every user, so I'd like to limit it if I can, or at least explain exactly what's going on and why shrinking this is impossible.

I'll keep digging and update when I have more 😃. Let me know if you work out any clues on your end in the meantime!

@kamko
Copy link
Contributor Author

kamko commented Jan 3, 2022

Unfortunately didn't have time to look into this.

It's really weird that keeping all shaded classes is not enough for this to work.
Did you also try to exclude only ASM or only bytebuddy?

I think that increasing the JAR size by 1.25MB (I know that it's 70% 😄) hurts but the installer is at 80MB already (at least for win). If we still try to minimize the bytebuddy/asm code these kind of problems could bite us again in the future. Although I understand the struggle as not everyone is using this and would be penalized by having to download more...

This allows us to limit the extra entry points to just the agent code.
It does increase the JAR size by about 0.1MB (now 2.3MB in total) but it
should be significantly less fragile and it's still much smaller than
the raw JAR (4.8MB). 30% size increase, but at least half due to a real
increase in size of ByteBuddy itself.
@pimterry
Copy link
Member

pimterry commented Jan 4, 2022

I think that increasing the JAR size by 1.25MB (I know that it's 70% 😄) hurts but the installer is at 80MB already (at least for win).

True, but these kind of things add up. And this also affects server updates (currently ~30MB) and this JAR gets copied into Docker containers when they're intercepted too (as part of a payload of about 10MB total). It's not critical, but I would like to avoid letting any of this creep up too much.

Anyway, I've managed to pull the size down a bit further (to 2.3MB) and simplify the config en route by disabling R8's automated optimizations entirely. Looks like those were only saving 0.1MB anyway, and hopefully that'll make this much less fragile in future generally, since it now mutates the code far less. I've pushed that change here.

I think this'll do for now. I'm going to merge this and release as 1.3.0 in just a sec. I've also fixed the annoying R8 build warnings from Akka & Scala on main, so hopefully it'll be easier to investigate and debug all any similar issues like this in future.

Thanks again for all your help and hard work on this! Really glad we've managed to get it working 😄

@pimterry pimterry merged commit 00fb9be into httptoolkit:main Jan 4, 2022
@pimterry pimterry changed the title [WIP] Update Byte Buddy to 1.12.6 (fixes #7) Update Byte Buddy to 1.12.6 (fixes #7) Jan 4, 2022
@kamko
Copy link
Contributor Author

kamko commented Jan 4, 2022

Great!, good job :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants